…change

Defer findings #4 (isGitRepo case-sensitivity) and #5 (GitChecker seam) out
of this PR. Restores the original exec-based isGitRepo and the New(store)
constructor; removes git.go, git_test.go, and the test-only export shims. The
error-standardization and other findings are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ayers (#95) (#96)

* refactor(backend): LLD maintainability fixes in controllers/service layers

Addresses the high + medium severity findings from the LLD review of
backend/internal/httpd and backend/internal/service (#95):

1. Controllers no longer import internal/session_manager. Session sentinel
   errors are now *domain.ServiceError values carrying their own HTTP mapping,
   so the controller translates them with one generic errors.As — no
   cross-package sentinel imports.
2. One error pattern across services: project.Error is now an alias of the
   shared domain.ServiceError, and session_manager sentinels use it too. A
   single writeServiceError replaces the per-resource error switches.
3. Clean-orchestrator business logic moved out of the controller into
   session.Service.SpawnOrchestrator(ctx, projectID, clean).
4. isGitRepo no longer treats case-different paths as equal on case-sensitive
   filesystems; case-insensitive compare is gated to darwin/windows via samePath.
5. Project repo check sits behind an injectable GitChecker, so the service is
   testable without a real git binary.
6. httpd exports only the production constructors (NewWithDeps,
   NewRouterWithControl); the 3 test-only wrappers are removed and the
   "router with empty deps" convenience moved to an unexported test helper.

Closes #95

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(backend): standardize service errors on internal/httpd/errors

Replace the domain.ServiceError approach with a REST-API-scoped error package
and a single envelope renderer, per review feedback:

- Add internal/httpd/errors (package errors, aliased apierr): one structured
  Error type with semantic Kinds (Internal/Invalid/NotFound/Conflict) and
  constructors. Imports nothing, so any layer can depend on it.
- envelope.WriteError is now the single path from a service error to the wire
  APIError, and the only place a Kind becomes an HTTP status/word. The
  per-resource writeProjectError/writeSessionError translators are gone.
- Delete domain/errors.go (keeps domain pure of HTTP-flavored kinds) and
  service/project/errors.go (no per-service error files); services build
  errors inline via apierr constructors.
- session_manager sentinels are apierr.Error values (pointer identity still
  works with errors.Is).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* revert(backend): drop GitChecker seam and isGitRepo case-sensitivity change

Defer findings #4 (isGitRepo case-sensitivity) and #5 (GitChecker seam) out
of this PR. Restores the original exec-based isGitRepo and the New(store)
constructor; removes git.go, git_test.go, and the test-only export shims. The
error-standardization and other findings are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(session): translate engine errors to API errors at the facade

The session_manager is the internal command engine and must not depend on the
REST API error vocabulary. Revert its sentinels to plain errors.New values and
move the engine→API translation into the service/session facade (toAPIError),
which is the correct boundary. Controllers still see apierr.Error and never
import the engine; the engine no longer imports internal/httpd/errors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(session): tighten error comments to state what the code does

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* style(envelope): make KindInternal an explicit case in httpStatus

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(apierr): rename package, test SpawnOrchestrator, parity fixes

Address review feedback on PR #96:
- Rename internal/httpd/errors → internal/httpd/apierr (package apierr) so
  importers no longer alias around the stdlib errors package.
- Add a commander seam to session.Service and unit-test the relocated
  clean-orchestrator rule: clean=true kills all active orchestrators before
  spawning; clean=false spawns without kills.
- project.Add: wrap the UpsertProject store error in apierr.Internal for parity
  with its sibling paths (was a raw 500).
- Document that KindInternal is iota's zero value, so a zero-value Error
  defaults to 500.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

…viewer to worker harness

Per PR #197 review feedback:
- Reviewer agent posts its review to the PR itself, so remove the
  ports.PRReviewPoster port, the GitHub review poster, the submit HTTP
  route + DTO, and the service Submit method (#1, #4, #7).
- Trigger spawns the reviewer agent over the worker's worktree with its
  own review prompt, mirroring the session launch flow (resolve agent by
  harness -> argv -> runtime.Create) (#8, #9).
- Default reviewer harness reuses the worker's harness when supported,
  falling back to claude-code; reviewer config stays independent of the
  worker override (#5, #6).
- Drop the `ao review` CLI for this PR's scope (#2, #3).

Regenerated OpenAPI + TS types.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(review): configurable AO code review backend (V1)

Add per-project configurable code review of a worker's PR. A reviewer
agent runs one-shot over the worker's own worktree and posts its result
to the PR; the worker picks the feedback up through the existing SCM
observer review-nudge path.

- domain: ProjectConfig.reviewers (+ default reviewer harness), Review /
  ReviewRun types and verdict/status vocab.
- storage: review + review_run tables (0011), sqlc queries, store methods.
- service/review: rewrite the in-memory stub as a persisted ReviewService
  (Trigger/Submit/List) with a reviewer Runner over agent resolver +
  runtime; ports.PRReviewPoster implemented on the GitHub adapter.
- http: session-scoped routes POST /sessions/{id}/reviews/trigger,
  POST .../submit, GET .../reviews; regenerated OpenAPI + TS types.
- cli: ao review trigger|submit|list.
- frontend: adapt ReviewDashboard to the per-worker reviews API.

Closes #192

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): address review — drop submit/poster/CLI, default reviewer to worker harness

Per PR #197 review feedback:
- Reviewer agent posts its review to the PR itself, so remove the
  ports.PRReviewPoster port, the GitHub review poster, the submit HTTP
  route + DTO, and the service Submit method (#1, #4, #7).
- Trigger spawns the reviewer agent over the worker's worktree with its
  own review prompt, mirroring the session launch flow (resolve agent by
  harness -> argv -> runtime.Create) (#8, #9).
- Default reviewer harness reuses the worker's harness when supported,
  falling back to claude-code; reviewer config stays independent of the
  worker override (#5, #6).
- Drop the `ao review` CLI for this PR's scope (#2, #3).

Regenerated OpenAPI + TS types.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(review): restore ao review submit (records verdict+body in AO)

Per maintainer request, bring back `ao review submit`. AO records the
reviewer's verdict and body on the review_run and marks the pass complete;
it does not post to GitHub — the reviewer agent posts its review to the PR
itself.

- storage: add review_run.body (0011), persist via Insert/UpdateReviewRunResult.
- service: restore Submit (no SCM poster) storing verdict + body.
- http: restore POST /sessions/{id}/reviews/submit + SubmitReviewInput.
- cli: ao review submit [worker] --verdict --body (worker from arg/--session/$AO_REVIEW_WORKER).
- runner: reviewer prompt instructs posting to GitHub and recording via ao review submit.

Regenerated OpenAPI + TS types.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): move reviewer runner to its own package; sharpen prompt

Per PR #197 review:
- Move the concrete reviewer runner out of the service layer into a new
  internal/review_runner package (package reviewrunner), beside other
  orchestration packages like session_manager. The service keeps only the
  Runner interface + RunSpec it depends on; the agent-resolver + runtime
  launch flow lives in review_runner.
- Sharpen the reviewer prompt: tell the agent to diff against the PR base,
  focus on high-confidence findings, post via `gh pr review`, and record
  the result with `ao review submit`; review-only (no commits/edits).
- Add unit tests for the runner.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): simplify review_run schema; provider-agnostic reviewer prompt

Per PR #197 review:
- review_run: status default 'running' (drop 'pending'), drop CHECK
  constraints on status/verdict, drop the updated_at column and the
  session/iteration index. Propagated through queries, domain, store,
  service, and tests.
- Reviewer prompt no longer hardcodes GitHub/gh commands — it instructs the
  agent to use whatever review tooling the provider offers, keeping the
  flow extensible across SCM providers.

Regenerated sqlc + OpenAPI/TS.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): launch reviewer before persisting the run

Trigger now spawns the reviewer agent first and then writes the review_run
with a status derived from the launch outcome (running on success, failed
if it never started), instead of inserting a running row and correcting it
to failed afterwards.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): pluggable reviewer registry distinct from worker harnesses

Reviewers are now their own pluggable adapter set, separate from the worker
agent registry — adding a reviewer (claude-code today, greptile tomorrow) is
a one-line registration that does not widen the worker harness vocabulary,
and a worker harness does not automatically become a valid reviewer.

- domain.ReviewerHarness: a distinct vocabulary (AllReviewerHarnesses) with
  its own IsKnown; ReviewerConfig/Review/ReviewRun use it. ResolveReviewerHarness
  reuses the worker harness only when it is itself a supported reviewer, else
  falls back to claude-code.
- ports.Reviewer: a reviewer-specific contract (ReviewCommand → argv + env)
  that models one-shot / non-prompt CLIs natively instead of forcing every
  reviewer through the worker's interactive GetLaunchCommand(Prompt:...).
- internal/adapters/reviewer: a separate registry + resolver (mirrors the
  worker agent registry) with the claude-code reviewer adapter, which owns the
  review prompt and reuses the worker claude-code launch construction.
- review_runner resolves via the reviewer registry (not the worker
  AgentResolver) and merges AO_REVIEW_WORKER into the adapter's env.
- daemon wires the reviewer resolver. Registry/domain parity is test-enforced.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(review): cover run-scoped reviewer submit

* fix(api): update generated review submit schema

* refactor(review): split core engine (internal/review) from API service

Move the review orchestration (Trigger/Submit/List, run-id generation,
deps, RunSpec/Runner, sentinels) into a transport-independent core package
internal/review (Engine). internal/service/review is now a thin API-flow
boundary: the controller-facing Manager interface + a Service that delegates
to the engine + error re-exports.

This keeps the service layer to API concerns and lets the same engine back a
future in-process CLI trigger without going through HTTP. review_runner now
depends on the core package; daemon builds the engine and wraps it in the
service. No API/schema changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(review): commit-aware trigger, reviewer handle for UI, no env vars

Reworks the review trigger lifecycle and drops env-based coupling:

- review_run gains target_sha (the reviewed commit) and drops iteration.
  A repeat trigger for the same PR head short-circuits to the existing run.
- review gains reviewer_handle_id: the live reviewer pane's runtime handle,
  reused across passes and exposed in the reviews API so the UI can attach
  its terminal over /mux.
- Trigger flow: if a live reviewer pane exists and a new commit arrived,
  message it to re-review; otherwise spawn a fresh reviewer. The run is
  recorded only after the reviewer is launched.
- No environment variables: the reviewer adapter embeds the explicit
  `ao review submit --session <w> --run <id>` command in the spawn prompt
  and the re-review message. CLI submit requires --run/--session (no env
  fallbacks).
- Merge review_runner into internal/review as a Launcher (spawn/notify/alive).
- Trigger returns 201 for a new pass, 200 when reusing an existing run.

Regenerated sqlc + OpenAPI/TS.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): author the reviewer prompt centrally, not in the adapter

Mirror the worker model (session_manager builds the prompt; adapters just
place it via LaunchConfig.Prompt). The reviewer prompt now lives in
internal/review/prompt.go and is passed through ports.ReviewInvocation.Prompt;
the claude-code reviewer adapter just feeds inv.Prompt to its launch command
and returns it as the re-review message. One-shot CLI reviewers may ignore it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): split reviewer prompt into system+task, mirroring buildSpawnTexts

Mirror session_manager.buildSpawnTexts for the reviewer: a standing role goes
in the system prompt, the per-pass task (PR/commit + exact `ao review submit`
command) goes in the user prompt. internal/review/prompt.go now returns
(prompt, systemPrompt); both flow through ports.ReviewInvocation and the
claude-code adapter places them via LaunchConfig{Prompt, SystemPrompt}. The
re-review message reuses the per-pass prompt (role already established in the
running pane).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Vaibhaav <user@example.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>